Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables parallel nc_accept() usage by making listening sockets non-blocking and refactoring the accept/poll logic to be safe across multiple server accept threads. It also adds a dedicated concurrency-focused test and bumps the library micro version.
Changes:
- Refactor server-side accept to poll/accept without per-bind mutexes by relying on non-blocking listening sockets.
- Remove
bind_lock/pollinstate from internal bind structures and update call-home accept accordingly. - Add
test_server_threadto validate parallel accept behavior and fix minor TLS errno checks.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_server_thread.c |
Adds a new cmocka test exercising parallel server accepts with multiple server/client threads. |
tests/CMakeLists.txt |
Registers the new test_server_thread test target. |
src/session_server.c |
Makes listening sockets non-blocking and introduces new pollfd-based concurrent accept helpers. |
src/session_p.h |
Removes obsolete bind fields/locks and updates internal accept API declarations/comments. |
src/session_client.c |
Updates call-home accept to use the new accept helper and removes the client CH bind lock. |
src/session_client_tls.c |
Minor TLS handshake loop cleanup (uses the correct socket variable). |
src/session_mbedtls.c |
Fixes errno checks (assignment → comparison) for WANT_READ/WANT_WRITE handling. |
src/server_config.c |
Removes endpoint bind-lock initialization/destruction now that it’s no longer used. |
CMakeLists.txt |
Bumps micro version/soversion to reflect internal changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Roytak
force-pushed
the
non-blocking-accept
branch
2 times, most recently
from
32b6eae to
7cc53bb
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the server-side accept path to support parallel nc_accept() calls (by removing per-endpoint poll/accept locking and making listening sockets non-blocking), and adds a dedicated test to validate multi-threaded accept behavior.
Changes:
- Reworks server accept logic to poll all listening binds and safely accept in multi-threaded scenarios.
- Makes listening sockets (INET/UNIX) non-blocking and adjusts related accept/host-logging helpers.
- Adds
test_server_threadCMocka test and wires it into the tests CMake configuration; also fixes anerrnocomparison bug in the mbedTLS backend.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_server_thread.c |
New multi-threaded accept/timeout regression test for parallel server accept threads. |
tests/CMakeLists.txt |
Registers the new test_server_thread test target. |
src/session_server.c |
Implements new pollfds-based accept flow; makes listening sockets non-blocking; refactors client host/log handling. |
src/session_p.h |
Updates internal structs/prototypes to match the new accept implementation (removes bind lock/pollin). |
src/session_mbedtls.c |
Fixes errno handling (== vs =) in TLS send/recv wrappers. |
src/session_client.c |
Updates Call Home accept path to use the new nc_server_ch_accept_binds() API and removes the old bind lock usage. |
src/session_client_tls.c |
Simplifies TLS handshake loop to use the actual socket variable. |
src/server_config.c |
Removes initialization/destruction of the removed endpoint bind lock. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Allow accepting new sessions from multiple threads. Made server sockets non-blocking.
Tests accepting connections with multiple threads with different timeouts.
Roytak
force-pushed
the
non-blocking-accept
branch
from
7cc53bb to
ac9609a
Compare
Truly allow accepting new sessions from multiple threads.